Skip to content

Fix phpstan/phpstan#13921: spurious error nullCoalesce.offset#5133

Closed
phpstan-bot wants to merge 1 commit intophpstan:2.1.xfrom
phpstan-bot:create-pull-request/patch-mex607o
Closed

Fix phpstan/phpstan#13921: spurious error nullCoalesce.offset#5133
phpstan-bot wants to merge 1 commit intophpstan:2.1.xfrom
phpstan-bot:create-pull-request/patch-mex607o

Conversation

@phpstan-bot
Copy link
Copy Markdown
Collaborator

Summary

Fixes a spurious nullCoalesce.offset error where $x[0] ?? null was incorrectly flagged as "always exists and is not nullable" after $x[0]['bar'] ?? null when the inner array type contained nullable values (e.g., list<array<?string>>).

Changes

  • Added sideEffectSave flag to EnsuredNonNullabilityResultExpression to distinguish target saves (where null was actually removed) from side-effect saves (where the type was changed by recursive parent narrowing in specifyExpressionType)
  • Modified ensureShallowNonNullability in src/Analyser/NodeScopeResolver.php to mark parent saves and early-return saves as side-effect saves, and track whether expressions existed in the original scope
  • Modified revertNonNullability in src/Analyser/NodeScopeResolver.php to clean up spurious expression types after reverting: side-effect saves that didn't exist in the original scope are unset
  • Added unsetExpressionType method to src/Analyser/MutatingScope.php for removing expression type holders without recursive side effects
  • New regression test in tests/PHPStan/Rules/Variables/data/bug-13921.php and test method in NullCoalesceRuleTest

Root cause

When processing $x[0]['bar'] ?? null, ensureNonNullability calls specifyExpressionType to remove null from $x[0]['bar']. This method recursively narrows parent array types as a side effect, creating expression type holders for $x[0] and $x with Yes certainty. After processing, revertNonNullability restores the saved types using specifyExpressionType, which again creates recursive side effects. While ancestor types ($x) are properly restored by later entries in the save list, intermediate expressions ($x[0]) retain spurious Yes-certainty expression type holders.

The IssetCheck rule checks $scope->hasExpressionType($expr)->yes() as an alternative to $type->hasOffsetValueType($dimType)->yes(). The spurious expression type for $x[0] made this check return true, causing the false positive on $x[0] ?? null.

The fix distinguishes between target saves (the actual expression whose null was removed) and side-effect saves (expressions changed as a side effect of recursive parent narrowing). After reverting, side-effect saves that didn't have expression types in the original scope are cleaned up.

Test

Added regression test tests/PHPStan/Rules/Variables/data/bug-13921.php that reproduces the exact scenario from the issue: a function with list<array<?string>> parameter where $x[0]['bar'] ?? null is followed by $x[0] ?? null. The test expects no errors (empty expected errors array).

Fixes phpstan/phpstan#13921

…types

- Fixed false positive where `$x[0] ?? null` was flagged as unnecessary after
  `$x[0]['bar'] ?? null` when the inner type was nullable (e.g. list<array<?string>>)
- Root cause: revertNonNullability used specifyExpressionType which recursively
  creates expression type holders for parent arrays as side effects, leaving
  spurious Yes-certainty expression types for intermediate array dim fetches
- Added sideEffectSave flag to EnsuredNonNullabilityResultExpression to distinguish
  target saves (null actually removed) from side-effect saves (type changed by
  recursive parent narrowing)
- After reverting, unset expression types for side-effect saves that didn't exist
  in the original scope
- Added unsetExpressionType method to MutatingScope
- New regression test in tests/PHPStan/Rules/Variables/data/bug-13921.php

Closes phpstan/phpstan#13921
@staabm staabm closed this Mar 8, 2026
@VincentLanglet VincentLanglet deleted the create-pull-request/patch-mex607o branch March 8, 2026 12:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants